Skip to content

fix: process page HTML outside PHP's output-buffer display handler - #1132

Open
selul wants to merge 4 commits into
developmentfrom
fix/ob-handler-capture-at-shutdown
Open

fix: process page HTML outside PHP's output-buffer display handler#1132
selul wants to merge 4 commits into
developmentfrom
fix/ob-handler-capture-at-shutdown

Conversation

@selul

@selul selul commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Production sites crashed with Optml_Manager::replace_content(): Cannot use output buffering in output buffering display handlers (issue #1126). The full-page replacement now runs outside PHP's display-handler context, where nested output buffering is legal and fatal errors keep their real message.

What changed

  • Capture bufferprocess_template_redirect_content() starts a capture buffer whose handler is a named method (handle_buffer_fallback). When a third party flushes the buffer early, the handler passes the content through unprocessed and logs it: running the filter graph inside a PHP display handler would turn any third-party ob_*() call into an uncatchable engine fatal — the crash this rework removes. Only the legacy mode keeps in-handler processing.

  • close_buffer() — flushes third-party buffers stacked above ours, captures our own buffer, processes the HTML in normal execution context, and echoes the result.

  • Buffer ownership — before: ob_end_flush() popped whichever buffer was on top. After: we only consume the buffer we started, verified by nesting level and handler identity (ob_get_status()['name'] must report Optml_Manager::handle_buffer_fallback), so a foreign buffer at our recorded level is never captured or closed.

  • close_final_buffer() — a re-armed buffer captures output echoed by later shutdown callbacks and processes it at shutdown priority PHP_INT_MAX.

  • do_url_replacement() — replaces extracted URLs in one pass per size-bounded chunk instead of one full-page preg_replace() per URL. Benchmark on a 1.2 MB page: 300 URLs in 4 ms instead of 77 ms, 900 URLs in 13 ms instead of 216 ms. Peak memory stays the same; the win is CPU time and allocation churn. Chunks flush at 200 URLs or 24KB of quoted pattern, so the compiled regex stays within PCRE's ~64KB limit even for kilobyte-long signed CDN URLs.

  • Escape hatch — return false from the new optml_capture_at_shutdown filter to restore the legacy in-handler flow.

Note

The crash location in the telemetry (FormatProperty.php:1) was an artifact. PHP masks a fatal error inside a display handler with the "Cannot use output buffering" message and an unrelated location. After this change, such fatals report their real message and location.

Shutdown flow

flowchart LR
    A[Changed:<br/>page renders into<br/>plain capture buffer]:::changed --> B[shutdown:<br/>close_buffer]
    B --> C[New:<br/>flush third-party<br/>buffers above]:::added
    C --> D{New:<br/>our buffer<br/>intact?}:::added
    D -- Yes --> E[Changed:<br/>process HTML<br/>outside handler]:::changed
    E --> F[Echo optimized page]
    F --> G[New:<br/>re-arm for late<br/>shutdown output]:::added
    D -- No --> H[Stand down:<br/>early-flushed content<br/>passed through, logged]

    classDef added fill:#1a7f37,color:#fff,stroke:#116329,stroke-width:3px
    classDef changed fill:#9a6700,color:#fff,stroke:#5c3d00,stroke-width:3px,stroke-dasharray:6 3
Loading

QA

  1. Connect the site in WP Admin → Media → Optimole. Create wp-content/mu-plugins/ob-probe.php with:

    <?php
    add_filter( 'optml_url_pre_process', function ( $html ) {
        ob_start();
        echo 'probe';
        ob_get_clean();
        return $html;
    } );

    Open any frontend page.

    Expect: the page renders, image URLs point to i.optimole.com, and wp-content/debug.log contains no Cannot use output buffering fatal. Without this fix, the page terminates with that fatal.

  2. Remove the mu-plugin. Add add_filter( 'optml_capture_at_shutdown', '__return_false' ); to the theme's functions.php. Open a frontend page and view the source.

    Expect: image URLs still point to i.optimole.com (legacy in-handler flow).

  3. Remove the filter. Install and activate TranslatePress. Translate a page that contains images and open the translated page.

    Expect: the translated text and i.optimole.com image URLs appear together.

🤖 Generated with Claude Code

Running replace_content() as the ob_start() display handler meant any
output-buffering call from third-party code hooked into our filters was
a fatal error, and any real fatal during processing (e.g. memory
exhaustion) was masked as "Cannot use output buffering in output
buffering display handlers" with a misleading crash location.

The buffer is now a plain capture: close_buffer() flushes third-party
buffers stacked above ours, captures our own by its recorded nesting
level (never popping someone else's buffer), processes the HTML in
normal execution context and re-arms the capture so late shutdown
output is still handled. The attached handler remains only as a
fallback that keeps the previous behavior when third-party code
flushes our buffer mid-request.

Also replaces the per-URL full-page preg_replace() loop with chunked
single-pass replacement to reduce peak memory on large pages, the
likely trigger of the masked production fatals.

Fixes #1126

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pirate-bot

pirate-bot commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Plugin build for f232081 is ready 🛎️!

selul and others added 3 commits September 1, 2026 13:19
A chunk of 200 very long URLs (e.g. signed CDN URLs with kilobyte-sized
query strings) could exceed PCRE's ~64KB compiled-pattern limit, failing
the whole chunk and leaving those URLs unreplaced. Chunks now flush when
the accumulated quoted pattern reaches 24KB, so compilation always
succeeds regardless of URL length, and a failed chunk is logged via
optml_log instead of being silently skipped.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Building every chunk's bookkeeping up front held all origin/replacement
maps in memory at once, which cost about 1MB extra on pages with
thousands of URLs. Each chunk is now applied as soon as it fills, so
only one chunk's bookkeeping exists at a time; peak memory is now at or
below the old per-URL loop at every scale.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
P1: the fallback handler no longer runs replace_content() when a third
party flushes our buffer early. The ob-in-handler fatal is an engine
E_ERROR that catch (Throwable) cannot intercept, so processing there
reintroduced the crash this rework removes; early-flushed content is
now passed through unprocessed and logged. Only the explicit legacy
mode (optml_capture_at_shutdown false) keeps in-handler processing.

P2: buffer ownership is now verified by handler identity, not nesting
level alone. The capture buffer uses a named method handler so
ob_get_status()['name'] reports Optml_Manager::handle_buffer_fallback,
and capture_and_process_buffer() refuses any buffer that does not
carry it — a foreign buffer at our recorded level is never consumed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants